I’m pretty strongly opposed to merging this when NixOS/nix#11098 hasn’t been addressed at all. The UX regression for macOS flakes users on 2.19+ is severe.

(I daily drive the newer versions, but I wouldn’t wish it on anyone else.)

I’m pretty strongly opposed to merging this when NixOS/nix#11098 hasn’t been addressed at all. The UX regression for macOS flakes users on 2.19+ is severe.

(I daily drive the newer versions, but I wouldn’t wish it on anyone else.)

Does this also happen on arm64 because I cannot confirm those performance numbers?

I can’t speak to whether @cigrainger is on AArch64. The performance regression isn’t as bad for me as described in the original post for that issue, but everything involving flakes is still noticeably and painfully slower. If it turns out to be x86_64-darwin only and it’s decided that regressing the UX for those users is acceptable, then fair enough. It may be that there are other relevant conditions, though. For example, just off the top of my head, you passed --store, which eliminates the factor of the Nix store being on a different APFS volume.

Ok. Would be great if you provide concrete numbers for different operations that you have seen being slower. I will try in a bit, if using the nix-daemon introduces a significant difference.

I’ll try to find the time to do some proper measurements. If other people can’t reproduce this then I don’t want to block on those grounds though; it just seemed like a lot of people found the upstream report for it to be a particularly specific thing.

Result of nixpkgs-review pr 335342 run on aarch64-darwin 1

I think we need to pin a few packages against older nix version. Some of them are using nix bindings.

I’ll try to find the time to do some proper measurements. If other people can’t reproduce this then I don’t want to block on those grounds though; it just seemed like a lot of people found the upstream report for it to be a particularly specific thing.

I can probably also get hold on a native x86_64-darwin machine, but that would take me a few days.

Result of nixpkgs-review pr 335342 run on aarch64-linux 1

nix-doc should have its plugin turned off entirely. it's not required if :doc knows how to do lambdas, which I've heard rumours has been implemented in some cppnix version? idk i have just been eating ice cream this whole time.

nixStatic should be a hard blocker on merging this. If it's broken since it broke upstream somehow, I guess y'all need to fix that and roll a release.

nixStatic should be a hard blocker on merging this. If it's broken since it broke upstream somehow, I guess y'all need to fix that and roll a release.

It's actually nixpkgs that is broken (perl-static), so we need to fix nixpkgs itself. nixStatic in the nix flake builds as it uses an older version of nixpkgs.

nix-doc should have its plugin turned off entirely. it's not required if :doc knows how to do lambdas, which I've heard rumours has been implemented in some cppnix version?

How does one verify that it works? We could otherwise also pin it to an old version and leave it to its maintainers to fix it.

nixStatic should be a hard blocker on merging this. If it's broken since it broke upstream somehow, I guess y'all need to fix that and roll a release.

It's actually nixpkgs that is broken (perl-static), so we need to fix nixpkgs itself. nixStatic in the nix flake builds as it uses an older version of nixpkgs.

Yep. Even on upstream/master the dependencies of nixStatic are broken. Not even libcurl does build

What I also found to be quite fast, especially w.r.t. to update is using nix's shallow clone feature:

    nixpkgs.url = "git+https://github.com/Mic92/nixpkgs?shallow=1";

I haven't bench-marked yet in depth but it feels to me that it also copies faster to the next store than the tarball cache.

devenv

cachix/devenv#1364 (comment)

Fixed in Devenv 1.1.0. It should not block this PR now.

We've restored the caching of github: and tarball-based store paths, avoiding expensive Nixpkgs copying DOC Python: python setup.py develop is not used anymore #11285

(BTW, after correcting the URL it looks like the backport hasn’t hit a 2.24 release yet, right?)

@emilazy I've fixed the link.
The change has been backported and released, but it doesn't show on the GitHub commit page.

$ git show --oneline ae486b2
ae486b291 Merge pull request #11446 from NixOS/mergify/bp/2.24-maintenance/pr-11285

$ git where ae486b2 
refs/remotes/upstream/2.24-maintenance
refs/remotes/upstream/latest-release
refs/tags/2.24.6
[ omitted a few lines ]

# Home Manager
programs.git.aliases.where = "for-each-ref '--format=%(refname)' --contains";

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nar-unpacking-vulnerability-post-mortem/52301/1

A nix-serve regression was mentioned in https://discourse.nixos.org/t/2024-09-16-nix-team-meeting-minutes-178-177/52270#bump-218-224-in-nixpkgs-4, but I didn't find relevant issues in https://github.com/NixOS/nix.

--- a/nix-serve.psgi
+++ b/nix-serve.psgi
@@ -22,6 +22,7 @@ BEGIN {
 my $app = sub {
     my $env = shift;
     my $path = $env->{PATH_INFO};
+    my $store = Nix::Store->new();
 
     if ($path eq "/nix-cache-info") {
         return [200, ['Content-Type' => 'text/plain'], ["StoreDir: $Nix::Config::storeDir\nWantMassQuery: 1\nPriority: 30\n"]];
@@ -29,9 +30,9 @@ my $app = sub {
 
     elsif ($path =~ /^\/([0-9a-z]+)\.narinfo$/) {
         my $hashPart = $1;
-        my $storePath = queryPathFromHashPart($hashPart);
+        my $storePath = $store->queryPathFromHashPart($hashPart);
         return [404, ['Content-Type' => 'text/plain'], ["No such path.\n"]] unless $storePath;
-        my ($deriver, $narHash, $time, $narSize, $refs, $sigs) = queryPathInfo($storePath, 1) or die;
+        my ($deriver, $narHash, $time, $narSize, $refs, $sigs) = $store->queryPathInfo($storePath, 1) or die;
         $narHash =~ /^sha256:(.*)/ or die;
         my $narHash2 = $1;
         die unless length($narHash2) == 52;
@@ -57,9 +58,9 @@ my $app = sub {
     elsif ($path =~ /^\/nar\/([0-9a-z]+)-([0-9a-z]+)\.nar$/) {
         my $hashPart = $1;
         my $expectedNarHash = $2;
-        my $storePath = queryPathFromHashPart($hashPart);
+        my $storePath = $store->queryPathFromHashPart($hashPart);
         return [404, ['Content-Type' => 'text/plain'], ["No such path.\n"]] unless $storePath;
-        my ($deriver, $narHash, $time, $narSize, $refs, $sigs) = queryPathInfo($storePath, 1) or die;
+        my ($deriver, $narHash, $time, $narSize, $refs, $sigs) = $store->queryPathInfo($storePath, 1) or die;
         return [404, ['Content-Type' => 'text/plain'], ["Incorrect NAR hash. Maybe the path has been recreated.\n"]]
             unless $narHash eq "sha256:$expectedNarHash";
         my $fh = new IO::Handle;
@@ -70,9 +71,9 @@ my $app = sub {
     # FIXME: remove soon.
     elsif ($path =~ /^\/nar\/([0-9a-z]+)\.nar$/) {
         my $hashPart = $1;
-        my $storePath = queryPathFromHashPart($hashPart);
+        my $storePath = $store->queryPathFromHashPart($hashPart);
         return [404, ['Content-Type' => 'text/plain'], ["No such path.\n"]] unless $storePath;
-        my ($deriver, $narHash, $time, $narSize, $refs) = queryPathInfo($storePath, 1) or die;
+        my ($deriver, $narHash, $time, $narSize, $refs) = $store->queryPathInfo($storePath, 1) or die;
         my $fh = new IO::Handle;
         open $fh, "-|", "nix", "dump-path", "--", $storePath;
         return [200, ['Content-Type' => 'text/plain', 'Content-Length' => $narSize], $fh];

Though still errors with

       > machine # [    7.426781] nix-daemon[852]: accepted connection from pid 838, user nix-serve
       > machine # [    7.464875] nix-serve-start[856]: warning: 'dump-path' is a deprecated alias for 'store dump-path'
       > machine # [    7.467184] nix-serve-start[856]: error: experimental Nix feature 'nix-command' is disabled; add '--extra-experimental-features nix-command' to enable it
       > machine #   0  229k    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0  229k    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
       > machine # curl: (18) end of response with 234672 bytes missing
       > machine: output:

With

--- a/nixos/tests/nix-serve.nix
+++ b/nixos/tests/nix-serve.nix
@@ -2,6 +2,7 @@ import ./make-test-python.nix ({ pkgs, ... }:
 {
   name = "nix-serve";
   nodes.machine = { pkgs, ... }: {
+    nix.settings.experimental-features = [ "nix-command" ];
     services.nix-serve.enable = true;
     environment.systemPackages = [
       pkgs.hello

test passes

Upstreamed one part edolstra/nix-serve#60

https://hydra.nixos.org/build/273031249/nixlog/6

Are we doing this again

oh no

holy crap it's the apparently intermittent bug that was reported in the nix matrix channel today. well i guess we have a repro now.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nar-unpacking-vulnerability-post-mortem/52301/29

https://github.com/NixOS/nix/blob/2.24.6/src/libexpr/eval.cc#L1965-L1973

I believe a bug might be present here but don't know enough c/c++, doesn't seem to have recent changes either though.

In particular my thought is that pos should be moved forward by l * sizeof(Value *) (the number of bytes written over to the new concated list) instead of by l (the number of elements copied over to the new list)

That mkList is one: NixOS/nix@fecff52 since we used to only run this test with Nix 2.18. Also, the probability of recurrence of this bug seems relatively low, so it's a bit hard to reproduce. I've run that test locally 10 times without luck.

Thanks for the explanation, don't think I can further help with this UnU

Dawn has a full core dump of the bug run in the matrix, if that's helpful to anyone figuring it out.